Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement QosProvider for C++ API. #480

Merged
merged 2 commits into from
Apr 10, 2024

Conversation

e-hndrks
Copy link
Contributor

@e-hndrks e-hndrks commented Apr 3, 2024

Add QosProvider functionality to C++ in accordance with the work done for eclipse-cyclonedds/cyclonedds#1955.

@e-hndrks e-hndrks force-pushed the master_erik branch 4 times, most recently from 31e744b to bd456c4 Compare April 9, 2024 14:50
Copy link
Contributor

@eboasson eboasson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two more thingummies, I'm afraid ...

@@ -118,6 +118,7 @@ steps:
cmake -DCMAKE_BUILD_TYPE=${BUILD_TYPE} \
-DCMAKE_INSTALL_PREFIX=install \
-DCMAKE_PREFIX_PATH="${BUILD_SOURCESDIRECTORY}/cyclonedds/build/install;${BUILD_SOURCESDIRECTORY}/iceoryx/build/install;${BUILD_SOURCESDIRECTORY}/googletest/build/install" \
-DCYCLONE_SOURCE_DIR="${BUILD_SOURCESDIRECTORY}/cyclonedds" \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect this is needed for including dds__sysdef_model.h in the test code, but it is really inelegant to require the C sources to be present to build the C++ binding — we never had that tight a relationship, and I don't think the QoS provider should change that.

Now, I haven't made an effort yet to find out which bits exactly are needed but it appears likely that only a fraction of what is in that file is needed. Before merging it like this, I'd like to know whether the bits that are needed can reasonably be installed by the core library. That would eliminate the need for this hack.

(Apologies for not spotting this earlier.)

@e-hndrks e-hndrks force-pushed the master_erik branch 2 times, most recently from dbabaec to 58ccd26 Compare April 10, 2024 10:58
@eboasson eboasson merged commit a5898db into eclipse-cyclonedds:master Apr 10, 2024
17 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants